-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix problem with named export axios
#5104
Conversation
8f2eb5d
to
35b8ab0
Compare
@jasonsaayman is there anything else you need me to do/explain? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
@@ -28,5 +28,6 @@ export { | |||
Cancel, | |||
isAxiosError, | |||
spread, | |||
toFormData | |||
toFormData, | |||
axios |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this should not be necessary you can already do this:
import { axios } from 'axios';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my updated PR description. You can check out my repro at akphi/issue-repo#12
There are 2 problems:
- For ESM
import
, I don't thinkimport { axios } from 'axios';
work - For TS, we need to update the typings to have the named export - I just updated the PR with this change
@jasonsaayman hi, could you please take a look at this again when you have time? Thanks! |
My team is also interested in this fix. |
Sure going to close this one in favour of #5162 |
@jasonsaayman I don't believe #5162 fixed the issue, as in akphi/issue-repo#12 I used |
@jasonsaayman Hi, could you please take a look at this again? Thanks! |
@jasonsaayman Could you let me know what else I can do for this PR? Do you want me to file a bug instead to be more formal and then resubmit my solution as a new PR? |
Instructions
Fixes #5101
I also chucked in a small fix for the default export to work with
NodeNext
, a bit of context heremicrosoft/TypeScript#49298
microsoft/TypeScript#50690
Basically, I will make it so people can import
axios
as a named import rather than having to do something like:UPDATE: I saw you have already taken care of #5101 by updating
exports
inpackage.json
, but the problem with named exports remains. Please see my repro here akphi/issue-repo#12I don't think
import { axios } from 'axios';
work at all.